fix: [Bug] NamespaceV2 is not working#10608
Conversation
When only namespaceV2 is set, topics and consumer/producer groups are not wrapped on the client side, because ClientConfig.getNamespace() — the single value used by withNamespace/withoutNamespace and every resource wrapping call site — only returns the deprecated V1 namespace, which stays empty. namespaceV2 was intended to be resolved server-side via the nsd/ns extension fields attached by NamespaceRpcHook, but the broker never consumes those fields; instead it derives the namespace from already-wrapped resource names (NamespaceUtil.getNamespaceFromResource). With neither side wrapping the resource, producers and consumers configured with different namespaces operate on the same physical topic, so a namespace can consume messages from another namespace. Make getNamespace() fall back to namespaceV2 when the V1 namespace is unset, so that resources are wrapped with the namespaceV2 value and isolated correctly. The V1 namespace still takes precedence when both are set. Signed-off-by: 王越 <1939455790@qq.com>
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
Summary
This PR fixes #9341 by making ClientConfig.getNamespace() fall back to namespaceV2 when the V1 namespace is not set, restoring namespace isolation for users who only configure namespaceV2.
Findings
-
[Info]
client/src/main/java/org/apache/rocketmq/client/ClientConfig.java:417-419— The fallback is correctly placed after the V1 check and before thenamesrvAddrparsing, preserving the intended priority: V1 > V2 > namesrvAddr-derived. The return is side-effect-free (does not mutatenamespaceor setnamespaceInitialized), which is consistent with how the V1 check on lines 413-415 behaves. -
[Info]
client/src/main/java/org/apache/rocketmq/client/ClientConfig.java:417-419— One subtle point: since the V2 fallback does not cache the result intonamespaceor setnamespaceInitialized = true, every call togetNamespace()will re-evaluate theStringUtils.isNotEmpty(namespaceV2)check. This is lightweight and matches the existing V1 pattern, so no action needed — just noting for awareness. -
[Info] The fix is purely client-side, which is the right approach here. Since the broker derives namespace from the resource name via
NamespaceUtil.getNamespaceFromResource, making the client correctly prefix resources with the V2 namespace value (e.g.,InstanceTest%TOPIC) is sufficient to restore isolation. TheNamespaceUtil.wrapNamespaceidempotency ensures safety when a proxy also performs its own resolution. -
[Info] Test coverage is solid — four test cases cover: basic fallback, wrapping with V2, unwrapping with V2, and V1 precedence. The idempotent wrapping test (
withNamespace("InstanceTest%resource")returning the same value) is a nice touch.
Suggestions
No blocking concerns. The change is minimal, well-targeted, and correctly addresses the root cause described in the issue.
Cross-repo Note
No changes needed in apache/rocketmq-clients — this fix is in the legacy Java client (client/ module), not the gRPC-based multi-language SDK.
Automated review by github-manager-bot
de41dce to
61197d6
Compare
RockteMQ-AI
left a comment
There was a problem hiding this comment.
Review by github-manager-bot
PR: Fix: NamespaceV2 is not working
Verdict: ✅ Approved
Summary
Fixes ClientConfig.getNamespace() to fall back to namespaceV2 when namespace (v1) is not set. Previously, if a user configured only namespaceV2 without namespace, the getNamespace() method would skip namespaceV2 entirely and attempt to parse the namespace from namesrvAddr, leading to incorrect behavior.
Analysis
Correctness ✅
- The fix adds the
namespaceV2fallback in the right position — after checkingnamespacebut before falling back to parsingnamesrvAddr. - The precedence is correct:
namespace(v1) >namespaceV2>namesrvAddrparsing.
Tests ✅
- 4 new test cases covering:
- Fallback to
namespaceV2when v1 is unset withNamespace/withoutNamespaceusingnamespaceV2- V1 precedence over V2
- Fallback to
- Good coverage of the fix.
Compatibility ✅
- No breaking changes. Existing behavior with
namespaceset is unchanged. - Only affects the previously broken case where only
namespaceV2was configured.
Performance ✅
- Simple string check, no performance impact.
Verdict
Clean fix with good test coverage. Addresses a real usability gap in the namespace v2 support.
What is the purpose of the change
Fixes #9341.
NamespaceV2does not isolate resources: a consumer in one namespace can consume messages produced by a producer in a different namespace.Brief changelog
namespaceV2was designed to be resolved server-side — the client attachesnsd/nsextension fields to each remoting request throughNamespaceRpcHook, expecting the server to wrap the resource. However, the broker never reads those fields; it derives the namespace from the (already-wrapped) resource name viaNamespaceUtil.getNamespaceFromResource. Meanwhile, all client-side resource wrapping (ClientConfig.withNamespace/withoutNamespaceand their call sites in the producer/consumer) is driven byClientConfig.getNamespace(), which only returns the deprecated V1namespaceand therefore stays empty when onlynamespaceV2is configured. The net effect is that neither side wraps the resource, so producers and consumers in different namespaces hit the same physical topic.This change makes
ClientConfig.getNamespace()fall back tonamespaceV2when the V1namespaceis not set (the V1 namespace still takes precedence when both are configured). BecausegetNamespace()is the single source of truth used by every resource wrapping/unwrapping call site, topics and groups are now correctly prefixed with thenamespaceV2value, restoring isolation:namespaceV2 = InstanceTest1) →InstanceTest1%NAMESPACE_TOPICnamespaceV2 = InstanceTest) →InstanceTest%NAMESPACE_TOPICNamespaceUtil.wrapNamespaceis idempotent (isAlreadyWithNamespace), so this is safe both for the direct-to-broker path (which ignoresnsd/ns) and for a proxy that performs its own resolution.Verifying this change
Added unit tests in
client/.../ClientConfigTest:getNamespace()returnsnamespaceV2when the V1 namespace is unset.withNamespace/withoutNamespacewrap and unwrap usingnamespaceV2, and wrapping is idempotent.namespacetakes precedence overnamespaceV2when both are set.Does this pull request potentially affect one of the following parts
ClientConfig.getNamespace()now falls back tonamespaceV2for resource wrapping when the V1 namespace is unset.ClientConfigbehavior.Documentation